Skip to content

[cuda.compute]: add CI job that builds c.parallel with thread sanitizer#9986

Merged
NaderAlAwar merged 17 commits into
NVIDIA:mainfrom
NaderAlAwar:add-cuda-compute-tsan-ft
Jul 25, 2026
Merged

[cuda.compute]: add CI job that builds c.parallel with thread sanitizer#9986
NaderAlAwar merged 17 commits into
NVIDIA:mainfrom
NaderAlAwar:add-cuda-compute-tsan-ft

Conversation

@NaderAlAwar

@NaderAlAwar NaderAlAwar commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

This also fixes a bug found by thread sanitizer

@copy-pr-bot

copy-pr-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@github-project-automation github-project-automation Bot moved this to Todo in CCCL Jul 16, 2026
@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Progress in CCCL Jul 16, 2026
@NaderAlAwar

Copy link
Copy Markdown
Contributor Author

/ok to test fff293a

The selector state is now per-call (stack-local), so the build-owned
selector ops carry state=nullptr. Drop the now-dead std::free(op.state)
calls in the destructor, cleanup path, and deserialize error handlers
(free(nullptr) is a harmless no-op but misleads readers into thinking the
build op still owns state), and fix the two comments accordingly. Every
op.code free is preserved.
@copy-pr-bot

copy-pr-bot Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@NaderAlAwar
NaderAlAwar force-pushed the add-cuda-compute-tsan-ft branch from 08fbce4 to d2280b9 Compare July 20, 2026 19:14
@NaderAlAwar

Copy link
Copy Markdown
Contributor Author

/ok to test d2280b9

@github-actions

This comment has been minimized.

@NaderAlAwar

Copy link
Copy Markdown
Contributor Author

/ok to test ac9237f

@github-actions

This comment has been minimized.

The python_tsan build links libcccl.c.parallel.so with -fsanitize=thread,
which the linker resolves via gcc-toolset-13's libtsan.so -- a package the
ci-wheel rockylinux8 image doesn't install with -gcc-c++, so the build failed
with 'cannot find -ltsan'. Install gcc-toolset-13-libtsan-devel when
CCCL_C_PARALLEL_SANITIZE_THREAD is set. Also pin the toolset version in one
gcc_toolset_version variable (the sanitizer runtime must match the compiler's
toolset, so they cannot drift apart).
Timing on the green CI run (29784468283) put the TSan lane's marginal cost at
~15.6 min/PR (a dedicated ~9.8 min instrumented build that can't share the
normal wheel producer, plus a ~5.8 min FT test). Worth it per-PR: the
segmented_sort data race was caught only by TSan, and it lives in the shared
build/launch machinery a PR can touch. Add python_tsan to pull_request
(keeping the nightly row, as the other python lanes do) and drop the stale
'Nightly-only' note.
@NaderAlAwar
NaderAlAwar force-pushed the add-cuda-compute-tsan-ft branch from 324ebc1 to f2931b5 Compare July 20, 2026 23:36
@NaderAlAwar

Copy link
Copy Markdown
Contributor Author

/ok to test 8e9f773

@NaderAlAwar NaderAlAwar changed the title [cuda.compute]: add nightly CI job that builds c.parallel with thread sanitizer [cuda.compute]: add CI job that builds c.parallel with thread sanitizer Jul 20, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@NaderAlAwar
NaderAlAwar marked this pull request as ready for review July 21, 2026 18:49
@NaderAlAwar
NaderAlAwar requested review from a team as code owners July 21, 2026 18:49
@NaderAlAwar
NaderAlAwar requested a review from shwina July 23, 2026 13:46
@NaderAlAwar

Copy link
Copy Markdown
Contributor Author

/ok to test 93cd393

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Comment thread c/parallel/CMakeLists.txt Outdated
Comment thread c/parallel/CMakeLists.txt Outdated
@oleksandr-pavlyk

Copy link
Copy Markdown
Contributor

Critical: the list of CI jobs does not contain ThreadSanitizer-labeled job, likely because it is not included in ci/project_files_and_dependencies.yaml. This causes python_tsan job to be pruned out from CI jobs for PRs. The comment in 'ci/matrix.yaml' suggests it was not intended

@NaderAlAwar

Copy link
Copy Markdown
Contributor Author

/ok to test bf5620d

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: bf3845e9-64cd-4e04-a0a6-fda2ff839905

📥 Commits

Reviewing files that changed from the base of the PR and between 93cd393 and bf5620d.

📒 Files selected for processing (3)
  • c/parallel/CMakeLists.txt
  • ci/matrix.yaml
  • ci/project_files_and_dependencies.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
  • c/parallel/CMakeLists.txt
  • ci/matrix.yaml

Comment thread ci/project_files_and_dependencies.yaml
@NaderAlAwar

Copy link
Copy Markdown
Contributor Author

/ok to test 7b03dcb

(cherry picked from commit be543f3)
@NaderAlAwar

Copy link
Copy Markdown
Contributor Author

/ok to test 1f09d02

# the linker resolves via the toolset's libtsan.so. That runtime lives in a
# separate package not pulled in by -gcc-c++, so install it for the TSan lane.
if [[ "${CCCL_C_PARALLEL_SANITIZE_THREAD:-}" =~ ^(1|true|TRUE|on|ON)$ ]]; then
/workspace/ci/util/retry.sh 5 30 dnf -y install "gcc-toolset-${gcc_toolset_version}-libtsan-devel"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any fedora based containers?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RockyLinux I see, I didn't know we had other distributions out of rapids.

@github-actions

This comment has been minimized.

@NaderAlAwar

Copy link
Copy Markdown
Contributor Author

/ok to test 850d3d8

@NaderAlAwar
NaderAlAwar enabled auto-merge (squash) July 25, 2026 00:10
@NaderAlAwar

Copy link
Copy Markdown
Contributor Author

/ok to test fb04700

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

🥳 CI Workflow Results

🟩 Finished in 2h 31m: Pass: 100%/524 | Total: 4d 08h | Max: 1h 01m | Hits: 100%/668691

See results here.

@NaderAlAwar
NaderAlAwar merged commit 0b0cba6 into NVIDIA:main Jul 25, 2026
1199 of 1206 checks passed
@github-project-automation github-project-automation Bot moved this from In Review to Done in CCCL Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants